Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complete refactor without changing API: Rewrite in TS, use Jest, update benchmarks #38

Merged
merged 18 commits into from
Apr 17, 2020

Conversation

hossam-magdy
Copy link
Contributor

@hossam-magdy hossam-magdy commented Mar 29, 2020

While keeping the API unchanged for the package consumers, the following changes are introduced:

  • rewrite the whole codebase in TypeScipt
  • use Jest with TS (instead of mocha+should+nyc)
  • configure eslint+prettier + TS parser
  • update travis.yml to use defaults and node 12
  • apply strict typing: tsconfig.noImplicitAny: true
  • write benchmarking shell script (./bench/_bench.sh)
  • update benchmark results (in README)
  • check solvable issues => solved \r breaks line, but it should not #22 and wrote test for it
  • ensure all pre-written tests are passing
  • ensure the success of yarn build, generated dist/ and npm pack/publish
  • add .vscode/launch.json ready-config for debugging tests
  • configure easily-accessible scripts: yarn build, yarn lint, yarn test, yarn bench

Noticed possible further improvements… for later

  • change the API so that functions accept options: object, instead of multi-signature/overriding functions (major release, breaking change)
  • fix the opposite case of issue \r breaks line, but it should not #22 stringify(parse(STRING_OF_ISSUE22)). A test for it is already added in this PR and skipped

@hossam-magdy hossam-magdy marked this pull request as ready for review March 30, 2020 23:41
@touv
Copy link
Collaborator

touv commented Apr 2, 2020

wahou, very big job !!
I need some time to tests with our production product.
I 'll give you a feedback as soon as possible.

@touv
Copy link
Collaborator

touv commented Apr 17, 2020

@hossam-magdy Can I merge this PR ?

@touv touv merged commit 3554c57 into Inist-CNRS:master Apr 17, 2020
@hossam-magdy
Copy link
Contributor Author

@touv Even though you already did 🎉 , Yes! 😄

@touv
Copy link
Collaborator

touv commented Apr 21, 2020

😋
thanks for your job !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants